MDEV-39675 mariadb_upgrade utility failed when path=nonexist#5106
Conversation
grooverdan
added
the
MariaDB Foundation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses MDEV-39675 by ensuring the mariadb_upgrade utility handles non-existent paths correctly. It introduces a regression test and updates the system tables fix script. Reviewer feedback suggests using the assignment operator for consistency in SQL statements and fixing a typo in the test case.
| --echo # | ||
|
|
||
| set @old_path=@@global.path; | ||
| set global path='non-existant'; |
There was a problem hiding this comment.
There is a minor typo in the string 'non-existant'. It should be 'non-existent'. Although this is a test value, correcting it improves the overall quality of the test suite. Note that the corresponding .result file should also be updated to avoid a test failure.
set global path='non-existent';
| # MDEV-39675 mariadb_upgrade utility failed when path=nonexist | ||
| # | ||
| set @old_path=@@global.path; | ||
| set global path='non-existant'; |
There was a problem hiding this comment.
Correcting the typo 'non-existant' to 'non-existent' to maintain consistency with the suggested change in the .test file.
set global path='non-existent';
|
I have one concern for adding Is there any use case when users use In this case, PATH variable won't exist in the server. This line will fail. To be more safe, is it better to just add |
|
@gengtianuiowa , I think either will work. When one downgrades from 12.3 to 11.8 and runs |
| set @old_path=@@global.path; | ||
| set global path='non-existant'; | ||
|
|
||
| --exec $MYSQL_UPGRADE --force --silent 2>&1 |
There was a problem hiding this comment.
are you sure, mariadb-upgrade was run and in particular has run that problematic code that failed before? There is nothing in the .result file to prove it
I'd suggest to make some innocuous change in the sys schema that mariadb-upgrade would correct and verify that it was corrected
There was a problem hiding this comment.
yes, without the script SQL change the mariadb-upgrade returns 1 and fails.
Sure, can do sys schema change in test.
Understood. Sounds like we always run I would still suggest using |
The upgrade SQL assumes the functions will resolve. With a non-default path this isn't true and results in: ERROR 1305 (42000) at line 3211: FUNCTION extract_schema_from_file_name does not exist Resolve by setting PATH = 'CURRENT_SCHEMA' for the upgrade. Idea thanks to Sergei Golubchik
The place where this is added the current database is I also found a If there's a revert for a reason, with the Thanks for taking the time for a comment. They where good questions that needed to be considered. |
3 participants
The upgrade SQL assumes the functions will resolve.
With a non-default path this isn't true and results in:
ERROR 1305 (42000) at line 3211: FUNCTION extract_schema_from_file_name does not exist
Resolve by setting PATH = 'CURRENT_SCHEMA' for the upgrade.
Idea thanks to Sergei Golubchik